Skip to content

Update/store retrieve list seperation#272

Closed
mateeullahmalik wants to merge 4 commits intomasterfrom
update/store-retrieve-list-seperation
Closed

Update/store retrieve list seperation#272
mateeullahmalik wants to merge 4 commits intomasterfrom
update/store-retrieve-list-seperation

Conversation

@mateeullahmalik
Copy link
Copy Markdown
Collaborator

No description provided.

@roomote-v0
Copy link
Copy Markdown

roomote-v0 Bot commented Feb 23, 2026

Rooviewer Clock   See task

Reviewed the latest commit (810de25 -- fix bootstrap routing gate). The change splits the routingAllowReady and routingAllowCount checks in eligibleForRouting and filterEligibleNodes so that routing stays permissive during bootstrap (before the first non-empty chain allowlist arrives) instead of blocking all peers. The logic is sound and the atomics ordering is safe. No new issues found.

Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR separates peer eligibility into two distinct allowlists—one for routing/read lookups and one for store/write replication—so postponed peers can still participate in routing while only active peers are used as write targets.

Changes:

  • Introduces a new storeAllowlist and eligibleForStore gating alongside updated routing/read eligibility semantics.
  • Updates store paths (storeToAlphaNodes, IterateBatchStore, batchStoreNetwork) to skip non-store-eligible peers and improves batch-store error reporting.
  • Adjusts bootstrap syncing to produce both routing (Active+Postponed) and store (Active-only) allowlists, and updates ping-success handling to reflect the new split.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
p2p/kademlia/dht.go Adds store allowlist + store eligibility checks; changes routing eligibility behavior; updates batch store candidate selection; disables delete/cleanup workers.
p2p/kademlia/bootstrap.go Extends chain bootstrap to compute routing vs store allowlists and wires them into the DHT.
p2p/kademlia/node_activity.go Updates ping success logic to add nodes to routing while only marking “Active” when store-eligible.
p2p/kademlia/dht_batch_store_test.go Updates expected error substring for the new batch-store failure message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread p2p/kademlia/dht.go Outdated
Comment thread p2p/kademlia/dht.go Outdated
Comment thread p2p/kademlia/dht.go
@mateeullahmalik mateeullahmalik marked this pull request as draft April 15, 2026 14:21
mateeullahmalik added a commit that referenced this pull request Apr 22, 2026
Extends PR #272's routing-vs-store allowlist split to cover the new
SUPERNODE_STATE_STORAGE_FULL introduced by lumera #113 (Everlight).

Policy:
- routing (reads)  = {ACTIVE, POSTPONED, STORAGE_FULL}
- store   (writes) = {ACTIVE}

STORAGE_FULL nodes continue to serve reads and earn payout, but must
not receive new STORE/BatchStore writes or be targeted by replication.

Changes:
- p2p/kademlia/supernode_state.go: SSoT helpers using chain's
  sntypes.SuperNodeState enum (no numeric literals), selfState cache,
  pruneIneligibleStorePeers, selfStoreEligible.
- bootstrap.go: use isRoutingEligibleState / isStoreEligibleState;
  record self-state during chain sync; call pruneIneligibleStorePeers
  after setStoreAllowlist so replication_info.Active is cleared
  eagerly for ineligible peers (closes ping-cadence window).
- network.go: STORE RPC self-guard for single handleStoreData (reject
  new-key write when self not store-eligible; replication of already-
  held key still permitted); BatchStoreData self-guard rejects when
  batch contains any genuinely new keys.
- dht.go: selfState atomic fields; defensive filterEligibleNodes on
  BatchRetrieve and BatchRetrieveStream closest-contact lists.
- dht_batch_store_test.go: accept new "no eligible store peers"
  error variant alongside legacy "no candidate nodes".
@mateeullahmalik
Copy link
Copy Markdown
Collaborator Author

mateeullahmalik commented Apr 22, 2026

Superseded by #284, which implements the same store/retrieve list separation but gated on the on-chain metrics_state (STORAGE_FULL / POSTPONED) introduced in lumera #113 and reported by #282. Closing in favor of the everlight-aligned approach.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants